-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lldb] Fix the DWARF index cache when index is partial. #118390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index. This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue.
|
@llvm/pr-subscribers-lldb Author: Greg Clayton (clayborg) ChangesThe ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index. This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue. Full diff: https://github.com/llvm/llvm-project/pull/118390.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 1220e6115a2a95..0be19ab29ef082 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -706,6 +706,11 @@ bool ManualDWARFIndex::Encode(DataEncoder &encoder) const {
return true;
}
+bool ManualDWARFIndex::IsPartial() const {
+ // If we have units or type units to skip, then this index is partial.
+ return !m_units_to_avoid.empty() || !m_type_sigs_to_avoid.empty();
+}
+
std::string ManualDWARFIndex::GetCacheKey() {
std::string key;
llvm::raw_string_ostream strm(key);
@@ -713,9 +718,26 @@ std::string ManualDWARFIndex::GetCacheKey() {
// module can have one object file as the main executable and might have
// another object file in a separate symbol file, or we might have a .dwo file
// that claims its module is the main executable.
+
+ // This class can be used to index all of the DWARF, or part of the DWARF
+ // when there is a .debug_names index where some compile or type units were
+ // built without .debug_names. So we need to know when we have a full manual
+ // DWARF index or a partial manual DWARF index and save them to different
+ // cache files. Before this fix we might end up debugging a binary with
+ // .debug_names where some of the compile or type units weren't indexed, and
+ // find an issue with the .debug_names tables (bugs or being incomplete), and
+ // then we disable loading the .debug_names by setting a setting in LLDB by
+ // running "settings set plugin.symbol-file.dwarf.ignore-file-indexes 0" in
+ // another LLDB instance. The problem arose when there was an index cache from
+ // a previous run where .debug_names was enabled and it had saved a cache file
+ // that only covered the missing compile and type units from the .debug_names,
+ // and with the setting that disables the loading of the cache files we would
+ // load partial cache index cache. So we need to pick a unique cache suffix
+ // name that indicates if the cache is partial or full to avoid this problem.
+ llvm::StringRef dwarf_index_suffix(IsPartial() ? "partial-" : "full-");
ObjectFile *objfile = m_dwarf->GetObjectFile();
strm << objfile->GetModule()->GetCacheKey() << "-dwarf-index-"
- << llvm::format_hex(objfile->GetCacheHash(), 10);
+ << dwarf_index_suffix << llvm::format_hex(objfile->GetCacheHash(), 10);
return key;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index d8c4a22ab21f7b..6a52c88a99220f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -168,6 +168,16 @@ class ManualDWARFIndex : public DWARFIndex {
const lldb::LanguageType cu_language,
IndexSet &set);
+ /// Return true if this manual DWARF index is covering only part of the DWARF.
+ ///
+ /// An instance of this class will be used to index all of the DWARF, but also
+ /// when we have .debug_names we will use one to index any compile or type
+ /// units that are not covered by the .debug_names table.
+ ///
+ /// \return
+ /// True if this index is a partial index, false otherwise.
+ bool IsPartial() const;
+
/// The DWARF file which we are indexing.
SymbolFileDWARF *m_dwarf;
/// Which dwarf units should we skip while building the index.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp
new file mode 100644
index 00000000000000..3e97c3fb1ebc24
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp
@@ -0,0 +1,62 @@
+// REQUIRES: lld
+
+// Test if we build a mixed binary where one .o file has a .debug_names and
+// another doesn't have one, that we save a full or partial index cache.
+// Previous versions of LLDB would have ManualDWARFIndex.cpp that would save out
+// an index cache to the same file regardless of wether the index cache was a
+// full DWARF manual index, or just the CUs and TUs that were missing from any
+// .debug_names tables. If the user had a .debug_names table and debugged once
+// with index caching enabled, then debugged again but set the setting to ignore
+// .debug_names ('settings set plugin.symbol-file.dwarf.ignore-file-indexes 1')
+// this could cause LLDB to load the index cache from the previous run which
+// was incomplete and it only contained the manually indexed DWARF from the run
+// where we used .debug_names, but it would now load it as if it were the
+// complete DWARF index.
+
+// Test that if we don't have .debug_names, that we save a full DWARF index.
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=1 -c %s -o %t.main.o
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=0 -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t.nonames
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.nonames.dwp
+// RUN: rm %t.main.dwo %t.foo.dwo
+// Run one time with the index cache enabled to populate the index cache. When
+// we populate the index cache we have to parse all of the DWARF debug info
+// and it is always available.
+// RUN: rm -rf %t.lldb-index-cache
+// RUN: %lldb \
+// RUN: -O 'settings set symbols.enable-lldb-index-cache true' \
+// RUN: -O 'settings set symbols.lldb-index-cache-path %t.lldb-index-cache' \
+// RUN: -O 'settings set target.preload-symbols true' \
+// RUN: %t.nonames -b
+
+// Make sure there is a file with "dwarf-index-full" in its filename
+// RUN: ls %t.lldb-index-cache | FileCheck %s -check-prefix=FULL
+// FULL: {{dwp-index-cache.cpp.tmp.nonames.*-dwarf-index-full-}}
+
+// Test that if we have one .o file with .debug_names and one without, that we
+// save a partial DWARF index.
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=1 -c %s -o %t.main.o -gpubnames
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=0 -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t.somenames
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.somenames.dwp
+// RUN: rm %t.main.dwo %t.foo.dwo
+// Run one time with the index cache enabled to populate the index cache. When
+// we populate the index cache we have to parse all of the DWARF debug info
+// and it is always available.
+// RUN: rm -rf %t.lldb-index-cache
+// RUN: %lldb \
+// RUN: -O 'settings set symbols.enable-lldb-index-cache true' \
+// RUN: -O 'settings set symbols.lldb-index-cache-path %t.lldb-index-cache' \
+// RUN: -O 'settings set target.preload-symbols true' \
+// RUN: %t.somenames -b
+
+// Make sure there is a file with "dwarf-index-full" in its filename
+// RUN: ls %t.lldb-index-cache | FileCheck %s -check-prefix=PARTIAL
+// PARTIAL: {{dwp-index-cache.cpp.tmp.somenames.*-dwarf-index-partial-}}
+
+#if MAIN
+extern int foo();
+int main() { return foo(); }
+#else
+int foo() { return 0; }
+#endif
|
| // RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.nonames.dwp | ||
| // RUN: rm %t.main.dwo %t.foo.dwo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these steps necessary. We should be able to build/save the index from dwo files alone, right?
| // RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.somenames.dwp | ||
| // RUN: rm %t.main.dwo %t.foo.dwo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
The ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index.
This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue.